Update method names #13
Conversation
Update method name of remove to delete to follow standards set here: https://github.com/howdyai/botkit/blob/master/docs/storage.md
|
Thanks for the PR and bringing this in line with the storage interface 😄 I added a few comments, nothing too big. Thanks again! |
src/index.js
Outdated
| @@ -35,6 +35,9 @@ module.exports = function(config) { | |||
| * @returns {{get: get, save: save, all: all, allById: allById}} | |||
| */ | |||
| function getStorageObj(client, namespace) { | |||
| var delete = function(id, cb) { | |||
There was a problem hiding this comment.
would you mind changing this to a function declaration (function delete(id, cb) {...})? That avoids any possible problems introduced by hoisting.
| client.hdel(namespace, [id], cb); | ||
| return delete(id, cb); | ||
| }, | ||
| delete: function(id, cb) { |
There was a problem hiding this comment.
Would you mind covering this new method with tests? You can just copy the tests for remove and rename the describe and method call, since these should behave identically.
| @@ -35,6 +35,9 @@ module.exports = function(config) { | |||
| * @returns {{get: get, save: save, all: all, allById: allById}} | |||
| */ | |||
| function getStorageObj(client, namespace) { | |||
| function delete(id, cb) { | |||
There was a problem hiding this comment.
I'm running this locally, turns out delete is a reserve word, could you change it to something else? Also you won't need a semi-colon after a function declaration. A little whitespace after the function would be nice too 😄 Otherwise things look great!
|
@Trudeaucj hey, thanks for a PR, there is already issue created for this issue #14. Please reference it in the body of this PR as |
|
Ref. to existing issue #14 |
Update method name of remove to delete to follow standards set here: https://github.com/howdyai/botkit/blob/master/docs/storage.md